- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1.4k
WIP: Interpolator class to cache interpolation matrix #4658
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
e38a11f    to
    5741db7      
    Compare
  
    | Codecov Report
 @@            Coverage Diff             @@
##           master    #4658      +/-   ##
==========================================
- Coverage   87.67%   87.65%   -0.03%     
==========================================
  Files         351      351              
  Lines       66609    68483    +1874     
  Branches    11548    12469     +921     
==========================================
+ Hits        58401    60026    +1625     
- Misses       5253     5504     +251     
+ Partials     2955     2953       -2 | 
| practically speaking, how much speed gain do you observe for EEG and for MEG by caching these matrices? | 
| computing one interpolation matrix takes in the order of seconds for me | 
| also for EEG? | 
| I haven't looked at EEG, only MEG. | 
| 
 I think the things we'd need to check would be: 
 I think this is enough to guarantee that the same interpolator can be used. | 
| okay. IMO, it makes sense only to cache  | 
| @jasmainak If we already go through the trouble of managing a cached object, is there an advantage to not caching the rest? I'm also thinking of epoch-wise interpolation, where you might want to apply an interpolator hundreds of times, so it might be preferable to store the highest level possible. | 
| @christianbrodbeck from what I understood, you only store the  My first thought had been to compute an  | 
| 
 If this is indeed the bottleneck, then for you would only ever need one of these for each MEG system, since the electrodes do not move. That would be nice. However, for EEG I doubt it's this simple since it uses some other interpolation mode. @christianbrodbeck this one is pretty old, perhaps we should close and reconsider the best way to cache the interpolation next time someone hits a need for it? | 
| I'll close since it seems unlikely that this will be implemented soon, and rebases are needed anyway | 
Addressing #4651
This should be the bare bones of the refactoring.
@larsoner is this enough to make it savable?
And what additional information should we store? The list of bad channels might be good, to make sure it's the same as
instbefore applying?